fix(webhook): remove dead diff code block that panics on nil namespace metadata#1976
Merged
oliverbaehler merged 1 commit intoJun 22, 2026
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR removes a redundant, unreachable metadata-diff block in userMetadataHandler.OnUpdate that both duplicated the real validation diff computation and could panic when cloning nil label/annotation maps.
Changes:
- Deleted the dead inline labels/annotations diff logic in
OnUpdatethat was immediately shadowed byuserMetadataForValidation(...). - Eliminated a confirmed nil-map write panic path (
maps.Clone(nil)followed by assignment) for namespaces without pre-existing labels/annotations.
…e metadata In userMetadataHandler.OnUpdate the per-request labels/annotations diff is computed twice. The inline block at the top clones oldNs metadata and diffs it against newNs, but its results are immediately shadowed by the `labels, annotations, err := userMetadataForValidation(...)` declaration in the `tnt.Spec.NamespaceOptions \!= nil` branch, which recomputes the same diff (via metadataForValidation) and is the only version actually passed to validateUserMetadata. The inline block is therefore dead code. It is also a panic: `maps.Clone(oldNs.GetAnnotations())` returns nil for a namespace with no annotations, and the following `annotations[key] = value` then panics with "assignment to entry in nil map" (same with labels). This happens when using features like NamespaceOptions.AdditionalMetadataList on a namespace that had none. The dead block and panic was introduced in a6927c5 (projectcapsule#1947). Signed-off-by: Cyril Jouve <jv.cyril@gmail.com>
jouve
force-pushed
the
pr/fix-namespace-webhook-nil-map
branch
from
8b8719b to
1d47c28
Compare
oliverbaehler
approved these changes
Jun 22, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
In userMetadataHandler.OnUpdate the per-request labels/annotations diff is computed twice. The inline block at the top clones oldNs metadata and diffs it against newNs, but its results are immediately shadowed by the
labels, annotations, err := userMetadataForValidation(...)declaration in thetnt.Spec.NamespaceOptions != nilbranch, which recomputes the same diff (via metadataForValidation) and is the only version actually passed to validateUserMetadata. The inline block is therefore dead code.It is also a panic:
maps.Clone(oldNs.GetAnnotations())returns nil for a namespace with no annotations, and the followingannotations[key] = valuethen panics with "assignment to entry in nil map" (same with labels).This happens when using features like NamespaceOptions.AdditionalMetadataList on a namespace that had none.
The dead block and panic was introduced in a6927c5 (#1947).